Skip to content

Support parameterized circuits in QASM3 export#159

Merged
doichanj merged 2 commits into
Qiskit:mainfrom
spital:qasm3-parameterized-export
Jun 16, 2026
Merged

Support parameterized circuits in QASM3 export#159
doichanj merged 2 commits into
Qiskit:mainfrom
spital:qasm3-parameterized-export

Conversation

@spital

@spital spital commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #146.

This moves the OpenQASM 3 export implementation out of QuantumCircuit into a dedicated exporter while keeping the public QuantumCircuit::to_qasm3() API. It also adds support for exporting parameterized circuits by emitting symbolic parameters as OpenQASM 3 input float[64] declarations.

What changed

  • Add a dedicated OpenQASM 3 exporter in src/circuit/qasm3_exporter.hpp and delegate QuantumCircuit::to_qasm3() to it.
  • Harvest symbolic identifiers from circuit parameter expressions and emit them as input float[64] declarations before registers and operations.
  • Allocate emitted input/register names so they avoid OpenQASM keywords, stdgates.inc globals, and custom gate definitions emitted by the exporter.
  • Preserve the existing flattened q register export behavior.
  • Emit valid OpenQASM 3 for controlled-U1, controlled-U3, and non-zero global phase (gphase).
  • Add regression tests for parameterized export, identifier collisions, custom-gate collisions, controlled-U definitions, and global phase.

Known limitations

The current Qiskit C API exposes the number of circuit parameter symbols but not an iterator over their names, so the exporter derives parameter identifiers from the string form returned by qk_param_str(). This supports normal symbolic parameters and expressions, but parameter names should be OpenQASM-identifier-like.

Some expression forms emitted by Qiskit's parameter string representation, such as power expressions (theta**2) or function calls (sin(theta)), are not currently round-trippable through qiskit_qasm3_import. The exporter preserves the expression spelling instead of rewriting it to a form that may change semantics.

Testing

  • make -C test/deps/qiskit c
  • cmake -S test -B /tmp/qisk146-hermes-review60-build
  • cmake --build /tmp/qisk146-hermes-review60-build -j20
  • ctest --test-dir /tmp/qisk146-hermes-review60-build --output-on-failure
  • /tmp/qisk146-hermes-review60-build/test_driver test_circuit
  • git diff --check origin/main...HEAD
  • Parser smoke tests with qiskit 2.4.1 / qiskit-qasm3-import 0.6.0 using qiskit.qasm3.loads() for reserved-name conflicts, keyword/constant-name conflicts, classical-register conflicts, controlled-U definitions, and global phase output.

AI assistance disclosure

I used Codex, Claude, and Hermes as AI assistants to help prototype, review, compare related approaches, and draft PR text. I manually reviewed the resulting diff, checked the live GitHub issue/PR state and unitaryHACK policy, refined the implementation, and ran the tests listed above locally. The PR is submitted with human oversight; any remaining mistakes are mine.

  • I have added the tests to cover my changes.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.

@spital spital force-pushed the qasm3-parameterized-export branch from 1e04495 to 10e1693 Compare June 10, 2026 05:38
@spital

spital commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main to resolve the merge conflicts.

The conflicts came from #160, which also touched to_qasm3 (physical-qubit $n references for transpiled circuits + skipping empty registers). Since this PR moves to_qasm3 into the new Qasm3Exporter, I ported that handling into the exporter so both behaviors are preserved. No functional changes to the parameterized-export feature itself.

All circuit tests pass, including both test_to_qasm3_parameterized (this PR) and test_to_qasm3_physical_qubits (from #160).

@spital spital force-pushed the qasm3-parameterized-export branch from 10e1693 to 719b5b2 Compare June 11, 2026 08:06
@spital spital force-pushed the qasm3-parameterized-export branch from 719b5b2 to 85275c1 Compare June 11, 2026 10:14
@spital

spital commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Updated this PR to address the maintainer request around QuantumCircuit::parameter_symbols() and the temporary duplicate-symbol limitation.

#146 (comment)

What changed:

  • Added QuantumCircuit::parameter_symbols() const as the requested temporary stub.

    • The Rust circuit remains authoritative.
    • The implementation traverses the circuit instructions and derives symbol names from the current C API parameter strings.
    • The derived list is cross-checked against qk_circuit_num_param_symbols(), so unsupported cases fail loudly instead of returning a misleading list.
    • I did not add QuantumCircuit::parameters().

  • Updated the OpenQASM 3 exporter to consume QuantumCircuit::parameter_symbols() for input declarations.

  • Implemented the temporary duplicate-name limitation.

    • Parameterized gate insertion now preflights against a copied Rust circuit.
    • If the C API reports QkExitCode_ParameterNameConflict, qiskit-cpp raises std::invalid_argument before mutating the real circuit.
    • This is covered for direct gate insertion, append(), compose(), and duplicate names within one multi-parameter gate.

  • Fixed a related compose() bug where the operation kind was read from the destination circuit instead of the source circuit.

  • Cleaned up the touched C API handle usage with RAII and made parameter_symbols() const.

Verification:

  • GCC 13.3: full ctest passes, 4/4 tests; test_circuit passes 20/20 subtests.
  • Clang 22.1.4: full ctest passes, 4/4 tests; test_circuit passes 20/20 subtests.
  • git diff --check is clean.
  • Release-note YAML parses.
  • Parser smoke tests with qiskit==2.4.1 / qiskit-qasm3-import==0.6.0 pass for parameter/register-name conflicts, CU1, CU3, global phase, and composed parameterized circuit output.

@spital spital mentioned this pull request Jun 12, 2026
Closed
doichanj
doichanj reviewed Jun 15, 2026
View reviewed changes
Comment thread src/circuit/quantumcircuit_def.hpp Outdated
reg_t qubit_map_; // qubit map caused by transpiling
std::vector<std::pair<uint_t, uint_t>> measure_map_; // a list of pair of qubit and clbit for measure

static void check_parameterized_gate_result(const QkExitCode result)

@doichanj doichanj Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think checking duplicate parameter symbols is not needed, just document this limitation is fine,
because overhead of copying quantum circuit here will be large for some cases

@spital spital Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, you are right. I removed the duplicate-symbol preflight copy/check path and documented the limitation instead. Distinct Parameter objects that share a name now collapse to one shared symbol, so the exported OpenQASM 3 remains valid but binds the colliding gates to the same input. I also updated the release note and replaced the rejection test with a test that documents this behavior. Please kindly review the update.

doichanj
doichanj requested changes Jun 15, 2026
View reviewed changes

@doichanj doichanj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this PR is well implemented and meets all the requirements in #146
One thing I would like to request is to remove duplicate checking

@spital

spital commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the kind words. PR #159 updated.

@spital spital requested a review from doichanj June 15, 2026 12:16
doichanj
doichanj approved these changes Jun 16, 2026
View reviewed changes

@doichanj doichanj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Thank you for your contribution.

@doichanj doichanj merged commit 6d90e8c into Qiskit:main Jun 16, 2026
6 checks passed
This was referenced Jun 16, 2026
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@doichanj doichanj doichanj approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

QASM3 exporter with parameterized circuit support

3 participants

@spital @doichanj @1ucian0